Skip to content
This repository has been archived by the owner on Jan 13, 2021. It is now read-only.

Fixes #52. Removed tracks filtering while fetching #53

Merged
merged 12 commits into from
May 10, 2019
Merged

Fixes #52. Removed tracks filtering while fetching #53

merged 12 commits into from
May 10, 2019

Conversation

nergal-perm
Copy link
Contributor

Also added the getThumbsUp() method to TrackApi to get only tracks liked by user.

FelixGail and others added 11 commits April 25, 2018 09:49
@FelixGail
Update README.md
a713779
@FelixGail
Merge pull request #32 from FelixGail/FelixGail-patch-1
25e0985 
Update README.md
@FelixGail
Update README.md
244066e
@FelixGail
Merge pull request #34 from FelixGail/FelixGail-patch-1
62ee45f 
Update README.md
@FelixGail
Merge pull request #39 from FelixGail/develop
9402f8c 
Release 0.3.5
@FelixGail
Fix broken link in CONTRIBUTING.md (#41)
314bbbe
@FelixGail
Release 0.3.6 (#51)
a89b92f 
* fix video retrieval (#43)

* add cache version to circle config (allow invalidating)

* update maven plugins

* update tika from vulnerable version 1.16 to 1.19.1

* update maven plugins

* update gson version to 2.8.5, add missing static to listresults inner class (#46)

* reformat code

* check if property test.track.playlist exists

* Release version 0.3.6
@nergal-perm
Get rid of unwanted symbol in javadoc
932b2d8 
Javadoc treats '>' symbol as unwanted, so it was replaced by 'greater
than' phrase.
@nergal-perm
Getting full media library
77aa2c7 
Premium users can add tracks to their libraries without purchasing them,
so there's no need to filter tracks by their storeId while fetching.
@nergal-perm
Added 'Thumbs Up' filter to TrackApi to get only tracks liked by user
a04ff87
@nergal-perm
Bumped up version, added a contibutor
126a4ee
@nergal-perm nergal-perm mentioned this pull request Nov 10, 2018
Closed
@FelixGail
Copy link
Owner

Thank you for the PR.
I do not remember why i added this filter - but i remember that there was a reason.
I will check this again in the next few days.

@nergal-perm
Copy link
Contributor Author

On second thought it seems to be better to create a new method to fetch all the library tracks (unfiltered), in order to keep existing method behaviour, because there are clients expecting this very result (filtered set of tracks). Let me know what you think about that, and I'll change the implementation accordingly.

@apoteet
Copy link

apoteet commented Jan 17, 2019 •
edited
Loading

@FelixGail Have you by chance checked if this filter was still needed? I'd love to have this functionality as well. I can fork this project and make my own change, but i'd rather stick with master.
Great repo, btw.

@nergal-perm Just my two cents, but it might be wise to make that change either a separate method, or treat this as a breaking change that requires a major release.

EDIT: After looking around a bit, it looks like storeid is needed to retrieve a streaming url from google play.
Track extends Signable, which has a urlFetcher method

gplaymusic/src/main/java/com/github/felixgail/gplaymusic/model/Signable.java

Lines 80 to 87 in a89b92f

protected URL urlFetcher(GPlayMusic api, StreamQuality quality,
Provider provider, Map<String, String> kwargs)
throws IOException {
Signature sig = getSignature();
if (getID().matches("^[TD]\\S*$")) {
return new URL(api.getService().getTrackLocationMJCK(api.getConfig().getAndroidID(), provider,
quality, sig.getSalt(), sig.getSignature(), getID(), kwargs
).execute().headers().get("Location"));

This method calls getID() in track which checks for storeID or UUID. So perhaps the intent was to only return songs that could be streamed from Google, which could be broken by this change.
I think that getting streamable and unstreamable tracks are both valid use cases, so maybe the best option is to introduce a method to the tracksAPI to get streamable and unstreamable songs. Which could mean changing the pagingHandler implementation in LibraryTrackCache to have a little more granularity in deciding how getAll tracks

gplaymusic/src/main/java/com/github/felixgail/gplaymusic/cache/LibraryTrackCache.java

Lines 17 to 32 in a89b92f

public LibraryTrackCache(GPlayMusic mainApi) {
pagingHandler = new PagingHandler<Track>() {
@Override
public ListResult<Track> getChunk(String nextPageToken) throws IOException {
return mainApi.getService()
.listTracks(new PagingRequest(nextPageToken, -1)).execute().body();
}
@Override
public List<Track> next() throws IOException {
return super.next().stream()
.filter(t -> !t.getStoreId().isPresent() && t.getUuid().isPresent())
.collect(Collectors.toList());
}
};
}

@FelixGail
Copy link
Owner

thats what i thought at first, too. But all songs should have an identifier, and getID should return the right identifier if there is one.

@FelixGail
Copy link
Owner

FelixGail commented Jan 23, 2019 •
edited
Loading

I created a snapshot of this version (felixgail:gplaymusic:0.3.7-SNAPSHOT).
The unit tests pass but are you willing to test if you run into any problems using this version?
Especially if songs without uuid or storeid are still playable. If they are there would be no need to split librarytracks into streamable and unstreamable songs

@nergal-perm
Copy link
Contributor Author

@FelixGail OK, I'll check it out

@FelixGail FelixGail changed the base branch from master to develop May 10, 2019 12:20
@FelixGail FelixGail merged commit ab8f2c6 into FelixGail:develop May 10, 2019
@nergal-perm nergal-perm deleted the bugfix/full-library branch May 13, 2019 09:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@FelixGail FelixGail FelixGail approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nergal-perm @FelixGail @apoteet